Upgrade async-profiler to 3.0 - #248
Conversation
|
I've also done manual testing using the elastiflix app and can confirm that inferred spans are still functional. |
SylvainJuge
left a comment
SylvainJuge
left a comment
There was a problem hiding this comment.
Nice work ! As usual I only have minor comments and questions.
| JfrParser jfrParser = new JfrParser(ByteBuffer.allocate(113), ByteBuffer.allocate(113)); | ||
| // Using a small buffer, but big enough to fit the largest string in the JFR file to test edge | ||
| // cases | ||
| JfrParser jfrParser = new JfrParser(ByteBuffer.allocate(368), ByteBuffer.allocate(368)); |
There was a problem hiding this comment.
[minor] where does this magic value comes from ? how to pick an appropriate value and maintain this in the future ?
There was a problem hiding this comment.
It is just a small value to trigger the edge cases in BufferdFile, like before.
In the new JFR-file the longest string was 368 characters, which therefore is the lower bound to not crash the parsing.
| throw new IllegalStateException( | ||
| "Unexpected sampling profiler error, everything else should work as expected. " | ||
| + "Please report to us with as many details, including OS and JVM details."); | ||
| "This implementation does not support reading JFR files containing multiple chunks"); |
There was a problem hiding this comment.
[question] does it means we could make it work (probably more efficiently) with >1 chunk per file in the future ?
There was a problem hiding this comment.
Async-profiler in the way we use it generates JFR files consisting only of a single chunk. Splitting those files into smaller chunks wouldn't make it more efficient to use, because then you'd end up with more duplications (e.g. duplicate stack traces).
…filer/JfrParser.java Co-authored-by: SylvainJuge <763082+SylvainJuge@users.noreply.github.com>
2 participants
Closes #173 .